-
-
Notifications
You must be signed in to change notification settings - Fork 697
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Solved Testimonial alignment issue #3509 #3510
Solved Testimonial alignment issue #3509 #3510
Conversation
Caution Review failedThe pull request is closed. WalkthroughThe pull request addresses a testimonial alignment issue on the AsyncAPI website by modifying CSS classes in the Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Welcome to AsyncAPI. Thanks a lot for creating your first pull request. Please check out our contributors guide useful for opening a pull request.
Keep in mind there are also other channels you can use to interact with AsyncAPI community. For more details check out this issue.
❌ Deploy Preview for asyncapi-website failed.Built without sensitive environment variables
|
✅ Deploy Preview for asyncapi-website ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3510 +/- ##
=======================================
Coverage 86.59% 86.59%
=======================================
Files 21 21
Lines 664 664
=======================================
Hits 575 575
Misses 89 89 ☔ View full report in Codecov by Sentry. |
We require all PRs to follow Conventional Commits specification.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.json
is excluded by!**/package-lock.json
📒 Files selected for processing (2)
package.json
(1 hunks)pages/[lang]/index.tsx
(1 hunks)
🔇 Additional comments (1)
pages/[lang]/index.tsx (1)
221-221
: Verify layout impact
Let's verify if this change affects the overall testimonial grid layout.
✅ Verification successful
Let me gather more specific information about the testimonial layout by examining the component and its usage.
Layout change is consistent with the design pattern
The padding change in the testimonial is part of a consistent grid layout where:
- Testimonials are arranged in a 2-column grid on medium screens (
md:grid-cols-2
) - Left column testimonials use
md:pr-10 lg:pr-16
- Right column testimonials use
md:pl-10 lg:pl-16
- The last testimonial (in question) has additional bottom padding (
md:pb-48 lg:pb-44
) which is intentional as it's the final element
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for other testimonial implementations to ensure consistency
# Look for other testimonial components or similar grid layouts
echo "Testimonial component usage:"
rg -l 'Testimonial'
# Look for similar grid patterns
echo -e "\nSimilar grid patterns:"
rg -B 2 -A 2 'md:grid-cols-2'
Length of output: 1796
Script:
#!/bin/bash
# Check the Testimonial component implementation and its usage
echo "Testimonial component implementation:"
cat components/Testimonial.tsx
echo -e "\nTestimonial usage in index page:"
rg -B 3 -A 3 '<Testimonial' pages/[lang]/index.tsx
Length of output: 4226
package.json
Outdated
@@ -82,7 +82,7 @@ | |||
"markdown-to-txt": "^2.0.1", | |||
"markdown-toc": "^1.2.0", | |||
"md5": "^2.3.0", | |||
"mermaid": "9.3.0", | |||
"mermaid": "^11.4.1", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codebase verification
Based on the initial search results, I can now provide a final assessment:
Major version update requires dedicated PR and testing
From the initial scan, we found:
- Mermaid is extensively used across 40+ documentation files
- There's a central configuration in
components/MDX/MDX.tsx
that initializes mermaid with specific settings - The update spans two major versions (9 to 11) which typically include breaking changes
Recommendations:
- Create a separate PR for the mermaid upgrade
- Test all documentation pages that contain diagrams (see file list from scan)
- Pay special attention to the configuration in
components/MDX/MDX.tsx
as it may need updates - Review the mermaid changelog for breaking changes between v9 and v11
🔗 Analysis chain
Unrelated dependency update detected
This PR's objective is to fix testimonial alignment, but includes a major version update of the mermaid package (9.3.0 to ^11.4.1). Major version updates often include breaking changes and should be handled in a separate PR.
Let's check for potential breaking changes:
Recommendations:
- Move this dependency update to a separate PR focused on the mermaid upgrade
- Test all diagrams with the new version before merging
- Review the mermaid changelog for breaking changes between v9 and v11
Let me gather more information about the specific mermaid diagrams in use to better assess the impact.
mermaid' markdown/
Check package-lock.json for the current version
echo -e "\nCurrent mermaid version in package-lock.json:"
rg '"mermaid":' package-lock.json
This will help us understand what types of diagrams are being used and confirm the current version before making a final assessment about the impact of the upgrade.
---
mermaid' markdown/
# Look for specific mermaid diagram types
echo -e "\nSpecific diagram types:"
rg -A 1 'graph [TDLR]|sequenceDiagram|classDiagram|stateDiagram|gantt|pie|flowchart' markdown/
# Check package-lock.json for version info
echo -e "\nMermaid version in package-lock.json:"
rg -A 3 '"mermaid":\s*{' package-lock.json
mermaid[\s\S]+?```" markdown/
Check current mermaid version
echo -e "\nCurrent mermaid version:"
grep -A 1 '"mermaid":' package-lock.json
</details>
<details>
<summary>🏁 Scripts executed</summary>
The following scripts were executed for the analysis:
Script:
```shell
#!/bin/bash
# Description: Search for mermaid usage in the codebase
# This will help assess the impact of the major version update
# Find files containing mermaid
echo "Files potentially using mermaid:"
rg -l 'mermaid'
# Look for mermaid configuration
echo -e "\nMermaid configuration:"
rg -A 5 'mermaid.*config|mermaid.*init'
Length of output: 2391
Script:
#!/bin/bash
# Search for mermaid diagram syntax in markdown files
echo "Mermaid diagram types in use:"
rg -B 1 -A 1 '
Length of output: 185
Script:
#!/bin/bash
# Search for mermaid diagram syntax in markdown files
echo "Mermaid diagram types in use:"
rg -B 1 -A 1 '
Length of output: 185
Script:
#!/bin/bash
# Search for mermaid codeblocks in markdown files
echo "Mermaid diagrams in use:"
rg -p "
Length of output: 175
pages/[lang]/index.tsx
Outdated
@@ -218,7 +218,7 @@ export default function HomePage() { | |||
authorDescription='Sr. Architect, Office of the CTO at Solace' | |||
/> | |||
<Testimonial | |||
className='md:pl-10 lg:pl-16' | |||
className='md:pl-10 md:pb-48 lg:pl-16 lg:pb-44' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Review padding values and approach
The added padding values (md:pb-48 = 12rem, lg:pb-44 = 11rem) seem unusually large for bottom spacing. This might indicate:
- A potential underlying layout issue that should be addressed differently
- Inconsistent treatment of testimonials, as only one testimonial is modified
Consider these alternatives:
- Investigate why this specific testimonial needs extra padding
- Apply consistent padding to all testimonials using a shared class
- Use grid or flexbox properties to ensure equal height testimonials
- className='md:pl-10 md:pb-48 lg:pl-16 lg:pb-44'
+ className='md:pl-10 lg:pl-16'
Then, add this CSS to your stylesheet:
.testimonials-grid {
@apply grid gap-6 md:grid-cols-2;
grid-auto-rows: 1fr; /* This ensures equal height */
}
And update the parent ul element:
- <ul className='mx-auto max-w-screen-xl md:grid md:grid-cols-2 md:px-6 lg:px-8'>
+ <ul className='testimonials-grid mx-auto max-w-screen-xl md:px-6 lg:px-8'>
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-3510--asyncapi-website.netlify.app/ |
Description
fixes #3509
This pr fixes the alignment issue in the testimonial section of the website.
Before the pr
After the pr
Related issue(s)
Summary by CodeRabbit